Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Object GoString() #86

Merged
merged 1 commit into from
Feb 22, 2021
Merged

Conversation

mildwonkey
Copy link
Contributor

The returned GoString() for a cty.ObjectWithOptionalAttrs was a bit off;
the returned slice of optional attributes was twice as long as it needed
to be due to a mismatch where the slice was created with make, but then
appended to (as opposed to indexed into).

I didn't see test coverage for Type GoString(), so I copied the test cases from Value's GoString() (fewer test cases, since I didn't need to differentiate between null and empty values). Here's the related test before the fix:

    --- FAIL: TestTypeGoString/cty.ObjectWithOptionalAttrs(map[string]cty.Type{"bar":cty.String,_"foo":cty.Bool},_[]string{"",_"bar"}) (0.00s)
        type_test.go:147: wrong result
            got:  cty.ObjectWithOptionalAttrs(map[string]cty.Type{"bar":cty.String, "foo":cty.Bool}, []string{"", "bar"})
            want: cty.ObjectWithOptionalAttrs(map[string]cty.Type{"bar":cty.String, "foo":cty.Bool}, []string{"bar"})

the returned slice of optional attributes was twice as long as it needed
to be due to a mismatch where the slice was created with make, but then
appended to (as opposed to indexed into).
@codecov
Copy link

codecov bot commented Feb 12, 2021

Codecov Report

Merging #86 (d5f83a1) into main (251ac0b) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #86   +/-   ##
=======================================
  Coverage   70.64%   70.64%           
=======================================
  Files          79       79           
  Lines        6547     6547           
=======================================
  Hits         4625     4625           
  Misses       1478     1478           
  Partials      444      444           
Impacted Files Coverage Δ
cty/object_type.go 78.26% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 251ac0b...d5f83a1. Read the comment docs.

@apparentlymart
Copy link
Collaborator

This looks great! Thanks for fixing this and sorry for the delay in reviewing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants